-
Notifications
You must be signed in to change notification settings - Fork 946
fix(watcher): use Watchman when available and reduce FSEvents usage #10115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Auto-detect Watchman on macOS and use it instead of FSEvents to avoid ~500 stream limit - Switch scope watcher to polling mode (300ms interval) to reduce FSEvents consumption - Enhance error message to recommend installing Watchman via 'brew install watchman'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses macOS FSEvents stream limitations that cause failures when running multiple Bit watchers. The solution implements automatic Watchman detection and fallback, along with reduced FSEvents usage.
Key Changes:
- Auto-detects and uses Watchman as the Parcel watcher backend on macOS when available
- Switches scope file watcher from FSEvents to polling mode (300ms interval)
- Improves error messaging to guide users toward installing Watchman
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scopes/workspace/watcher/watcher.ts | Adds Watchman detection and auto-configuration for Parcel watcher with new getParcelWatcherOptions() and isWatchmanAvailable() methods |
| scopes/workspace/watcher/fsevents-error.ts | Enhances error message to recommend Watchman installation via Homebrew |
| scopes/scope/scope/scope.main.runtime.ts | Switches scope file watching to polling mode to reduce FSEvents stream consumption |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
scopes/workspace/watcher/watcher.ts
Outdated
| return this.watchmanAvailable; | ||
| } | ||
| try { | ||
| execSync('watchman version', { stdio: 'ignore', timeout: 5000 }); |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The execSync call should include the shell: false option for security. While the command is hardcoded (not using user input), explicitly disabling shell interpretation is a security best practice to prevent command injection vulnerabilities.
Suggested fix:
execSync('watchman version', { stdio: 'ignore', timeout: 5000, shell: false });| execSync('watchman version', { stdio: 'ignore', timeout: 5000 }); | |
| execSync('watchman version', { stdio: 'ignore', timeout: 5000, shell: false }); |
scopes/workspace/watcher/watcher.ts
Outdated
| } | ||
|
|
||
| /** | ||
| * Check if Watchman is installed and running. |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states "Check if Watchman is installed and running" but the implementation only checks if the watchman version command executes successfully. This doesn't verify that the Watchman daemon is actually running and ready to watch files.
Consider either:
- Updating the comment to accurately reflect what's being checked: "Check if Watchman is installed"
- Or enhancing the check to verify the daemon is running (e.g., using
watchman get-socknamewhich requires the daemon to be active)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
scopes/workspace/watcher/watcher.ts
Outdated
| const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 }); | ||
| this.watchmanAvailable = result.status === 0; |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spawnSync call should include error handling for robustness. While the current logic correctly falls back to false when status !== 0, spawnSync can:
- Throw exceptions in rare cases (e.g., invalid options or system resource issues)
- Return
status: nullwhen the process times out or is terminated by a signal - Set
result.errorwhen the command fails to spawn
Without error handling, an exception would leave watchmanAvailable as null, causing repeated execution attempts. Consider wrapping the check in a try-catch and checking for result.error:
private isWatchmanAvailable(): boolean {
if (this.watchmanAvailable !== null) {
return this.watchmanAvailable;
}
try {
const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 });
if (result.error) {
this.logger.debug(`Watchman check failed: ${result.error.message}`);
this.watchmanAvailable = false;
} else {
this.watchmanAvailable = result.status === 0;
}
} catch (err) {
this.logger.debug(`Error checking Watchman availability: ${err}`);
this.watchmanAvailable = false;
}
return this.watchmanAvailable;
}| const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 }); | |
| this.watchmanAvailable = result.status === 0; | |
| try { | |
| const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 }); | |
| if (result.error) { | |
| this.logger.debug(`Watchman check failed: ${result.error.message}`); | |
| this.watchmanAvailable = false; | |
| } else { | |
| this.watchmanAvailable = result.status === 0; | |
| } | |
| } catch (err: any) { | |
| this.logger.debug(`Error checking Watchman availability: ${err}`); | |
| this.watchmanAvailable = false; | |
| } |
e2e/harmony/jest.e2e.ts
Outdated
| expect(output).to.have.string('✓ should pass'); | ||
| }); | ||
| it('bit build should show the passing component via Jest output', () => { | ||
| it.only('bit build should show the passing component via Jest output', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove only @davidfirst
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 }); | ||
| // Check for spawn errors (e.g., command not found) or non-zero exit status | ||
| this.watchmanAvailable = !result.error && result.status === 0; | ||
| } catch { |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider logging when Watchman detection fails or times out to help users debug watcher configuration issues. This would make it easier to diagnose problems like Watchman being installed but not responding properly.
try {
const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 });
this.watchmanAvailable = !result.error && result.status === 0;
if (!this.watchmanAvailable && result.error) {
this.logger.debug(`Watchman check failed: ${result.error.message}`);
}
} catch (err: any) {
this.logger.debug(`Watchman check threw exception: ${err.message}`);
this.watchmanAvailable = false;
}| } catch { | |
| if (!this.watchmanAvailable) { | |
| if (result.error) { | |
| this.logger.debug(`Watchman detection failed: ${result.error.message}`); | |
| } else { | |
| this.logger.debug(`Watchman detection failed: exited with status ${result.status}`); | |
| } | |
| } | |
| } catch (err: any) { | |
| this.logger.debug(`Watchman detection threw exception: ${err?.message ?? err}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import type { Event } from '@parcel/watcher'; | ||
| import type { Event, Options as ParcelWatcherOptions } from '@parcel/watcher'; | ||
| import ParcelWatcher from '@parcel/watcher'; | ||
| import { spawnSync } from 'child_process'; |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spawnSync import should be combined with the existing ChildProcess import from child_process at line 13 to maintain consistent import organization.
Consider changing line 13 to:
import type { ChildProcess } from 'child_process';
import { spawnSync } from 'child_process';or
import { spawnSync, type ChildProcess } from 'child_process';
On macOS, FSEvents has a system-wide limit of ~500 streams which causes "Error starting FSEvents stream" when running multiple Bit watchers across workspaces.
This PR:
brew install watchmanWhen Watchman is installed, it acts as a single daemon for all file watching, avoiding the FSEvents limit entirely.